Skip to content

fix(loader): reset per-load state so msg_ids don't leak between logs#20

Merged
Georacer merged 1 commit into
ArduPilot:mainfrom
dakejahl:fix/reset-state-per-load
May 22, 2026
Merged

fix(loader): reset per-load state so msg_ids don't leak between logs#20
Georacer merged 1 commit into
ArduPilot:mainfrom
dakejahl:fix/reset-state-per-load

Conversation

@dakejahl
Copy link
Copy Markdown
Contributor

Summary

All parser bookkeeping (has_fmt / has_fmtu / has_instance / instance_idx / instance_offset / formats / format_units / msg_id2name / msg_name2id / field_name2idx / messages_map / multipliers / units) lives in DataLoadAPBIN member variables, but the plugin instance is reused for every file load in a session and readDataFromFile never reset any of them. This patch clears them at the top of readDataFromFile.

Problem

ArduPilot assigns msg_ids dynamically per boot, so loading two logs in sequence can put a different message at the same id. The FMTU handler guards the # check with if (!has_instance[msg_id]), so a stale true from a previous file's message is never re-evaluated. When the next log puts a non-instanced message at that id, it gets treated as instanced and a byte from its payload (at the previous file's instance_offset) is read as the instance number.

Reproducer using two real ArduPilot logs from the same vehicle (one had msg_id 251 = SURF, the next had msg_id 251 = HEAT):

Before fix:
  After loading log_A.bin (msg_id 251 == SURF):
    HEAT series count: 5
  After loading log_B.bin (msg_id 251 == HEAT):
    HEAT series count: 308       <-- spurious /HEAT/#-102/...,
                                     /HEAT/#-105/..., etc.

After fix:
  After loading log_A.bin:  HEAT series count: 5
  After loading log_B.bin:  HEAT series count: 5

The accumulated messages_map also causes timestamps that already went through apply_multipliers + apply_timesync to be re-processed on the next load, producing far-future stray data points that compress the plot's useful range against one edge.

Solution

std::fill the bool/int arrays, value-initialize the log_Format / log_Format_Units structs, clear the strings in msg_id2name, and call .clear() on the maps. Verified the fix by loading the same two-log sequence via QPluginLoader and dumping the resulting PlotDataMapRef.

All parser bookkeeping lives in DataLoadAPBIN member variables
(has_fmt / has_fmtu / has_instance / instance_idx / instance_offset /
formats / format_units / msg_id2name / msg_name2id / field_name2idx /
messages_map / multipliers / units), but the plugin instance is reused
for every file load in a session and readDataFromFile never reset any of
them.

ArduPilot recycles non-static msg_ids per boot, so loading two logs in
sequence can put a different message at the same id. Because the FMTU
handler guards the '#' check with `if (!has_instance[msg_id])`, a stale
`true` from the previous file's message is never cleared. The next log's
message at that id gets treated as instanced and a byte from its payload
(at the previous file's instance_offset) is read as the instance number,
producing dozens of bogus "#N" children — e.g. HEAT exploded into 300+
spurious instances after a log where msg_id 251 was SURF.

The accumulated messages_map also caused already-multiplied timestamps
from earlier loads to be re-multiplied and re-time-synced, leaving a
stray data point on the far right of plots.

Clearing the state at the top of readDataFromFile fixes both.
@Georacer
Copy link
Copy Markdown
Contributor

Thank you for that! It has been a problem for so long!

Perhaps related to this, I have noticed that if you try to open a .bin after already loading one and a layout, it would take a VERY long time to load a hoard a bunch of RAM. Did you also notice by any chance if this got resolved as well?

@Georacer Georacer merged commit f2f5531 into ArduPilot:main May 22, 2026
1 check passed
@Georacer
Copy link
Copy Markdown
Contributor

It does seem to load the 2nd file quickly and correctly now. Very nice!

@dakejahl
Copy link
Copy Markdown
Contributor Author

Perhaps related to this, I have noticed that if you try to open a .bin after already loading one and a layout, it would take a VERY long time to load a hoard a bunch of RAM. Did you also notice by any chance if this got resolved as well?

I did not notice that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants